-
Notifications
You must be signed in to change notification settings - Fork 366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix recent CI errors in Neko and python 3.11 #1699
Conversation
I'm not quite sure what the goal is here, the 3.11 jobs are still failing because of their use of 3.11. Duplicating the job definition doesn't remove the jobs or change anything that I can tell. Is the intent here to stop running 3.11 tests? You would have to remove 3.11 from the list to do that. The primary workaround you can do while waiting on terra 0.22.4 (which should be out on Monday or Tuesday) is to set the version to 3.11.0 the error was introduced by 3.11.1 so explicitly setting the patch version should avoid this. The other option is to decouple the base aer test class from qiskit's base test class. We arguably shouldn't be inheriting a test class like this across a library boundary exactly for reasons like this. In general test code you want to be as flat and explicit as possible and defining a bunch of core behavior outside the local test suite will just cause trouble in the long run. (it's also been on my list for a while to stop exposing this as a public interface in terra, see: Qiskit/qiskit#6862 ) |
3cc32ba
to
8c72a2a
Compare
0017365
to
762c3a8
Compare
@mtreinish Thanks for your comments. I pinned python version to 3.11.0. |
7eb9736
to
688fab5
Compare
.github/workflows/neko.yml
Outdated
@@ -14,4 +14,7 @@ jobs: | |||
- uses: Qiskit/qiskit-neko@main | |||
with: | |||
test_selection: backend | |||
repo_install_command: pip install -e . | |||
repo_install_command: | | |||
pip install -r requirements-dev.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be needed, neko's tests have their own dev requirements (which get installed by the action definition) so we should only need to install aer from source to be able to run the tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtreinish as shown in https://github.com/hhorii/qiskit-aer/actions/runs/3912462608/jobs/6687134955, installation of qiskit-aer fails because neko does not include conan
in its requirements-dev.txt
, I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtreinish I confirmed that conan
command is installed but cmake does not recognize it as executable. In my local env, following logs are appeared in pip install -v .
but not in this CI.
changing mode of /tmp/pip-build-env-hya3x7mj/overlay/bin/conan to 755
changing mode of /tmp/pip-build-env-hya3x7mj/overlay/bin/conan_build_info to 755
changing mode of /tmp/pip-build-env-hya3x7mj/overlay/bin/conan_server to 755
Usually, pip
builds these commands in a temporary directory. I tried to change TMPDIR
in aer installation but errors remained.
I think that installing conan
manually before qiskit-aer installation in neko is a workaround. Could you give your thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is pointing to a wider issue in building from source. It looks like pip installing conan from pyproject.toml in that log but then cmake is not able to find it for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current pyproject.toml install conan
correctly in my local docker environment. I guess that it does not work well only in github actions for conan
.
releasenotes/notes/fix_ci_fails_due_to_python311-367ea7ca0abbfc2a.yaml
Outdated
Show resolved
Hide resolved
25471d3
to
b1bbafc
Compare
Now new terra 0.22.4 was released and 3.11.1 is no longer an issue. |
Summary
Tests for python 3.11.1 are failed until the next terra release. Neko is failed because
python -e .
fails. This PR provides workaround for them.Details and comments
This PR pins minor version of python 3.11 to 3.11.0 to pass all the tests.
Install Aer in Neko with
python .
(without-e
option) and removeqiskit_aer
directory to correctly refer the installed package.Resolve #1698 and #1695.